Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Odis Combiner to k8s #47

Merged
merged 95 commits into from
Oct 23, 2023
Merged

Odis Combiner to k8s #47

merged 95 commits into from
Oct 23, 2023

Conversation

aaronmgdr
Copy link
Member

@aaronmgdr aaronmgdr commented Sep 29, 2023 •
edited by alecps
Loading

Description

Migrate ODIS Combiner from firebase function to k8s. Add Prometheus metrics

Other changes

Tested

CI + e2e and load testing prior to release

Related issues

Backwards compatibility

Backwards compatible with clients thanks to proxying via the old Combiner. Not backwards compatible with old combiner monitoring and alerting / infrastructure

Documentation

NA

@changeset-bot
Copy link

changeset-bot bot commented Sep 29, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 6c46f2f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@aaronmgdr aaronmgdr mentioned this pull request Sep 29, 2023
Closed
@socket-security
Copy link

socket-security bot commented Sep 29, 2023 •
edited
Loading

New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
node-fetch 2.6.9 None +0 162 kB node-fetch-bot
prettier 2.8.7 None +0 11.2 MB prettier-bot
@opentelemetry/auto-instrumentations-node 0.38.0 environment +252 34.4 MB dyladan
jest-snapshot 29.7.0...29.5.0 None +54/-41 11.1 MB simenb
jest 29.7.0...29.5.0 None +96/-89 13 MB simenb
@types/express 4.17.18...4.17.17 None +28/-36 340 kB types
@types/node-fetch 2.6.6...2.6.4 None +2/-0 97.6 kB types
babel-jest 29.7.0...29.5.0 None +53/-45 10.9 MB simenb
@types/jest 29.5.5...29.5.4 None +11/-8 365 kB types
@babel/core 7.23.2...7.21.4 environment +31/-25 10.1 MB nicolo-ribaudo
@babel/runtime 7.23.2...7.21.0 None +1/-1 285 kB nicolo-ribaudo
@typescript-eslint/eslint-plugin 6.7.5...6.7.4 None +13/-12 7.39 MB jameshenry
@typescript-eslint/parser 6.7.5...6.7.4 None +10/-9 4.52 MB jameshenry
jest-circus 29.7.0...29.5.0 None +73/-62 12 MB simenb
@types/supertest 2.0.14...2.0.12 None +3/-3 44.7 kB types
eslint-plugin-prettier 5.0.1...5.0.0 None +5/-5 3.11 MB jounqin
eslint 8.51.0...8.50.0 None +4/-4 3.08 MB eslintbot
@opentelemetry/sdk-trace-web 1.17.1...1.15.2 None +0/-4 225 kB pichlermarc

🚮 Removed packages: @opentelemetry/[email protected], @opentelemetry/[email protected], @types/[email protected], @types/[email protected], [email protected], [email protected], [email protected]

@alvarof2 alvarof2 mentioned this pull request Oct 3, 2023
Merged
@alecps alecps marked this pull request as ready for review October 17, 2023 19:08
@alecps alecps requested a review from soloseng October 17, 2023 20:51
soloseng
soloseng approved these changes Oct 17, 2023
View reviewed changes
Copy link
Contributor

@soloseng soloseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments.

Few general questions:

  • Is this PR deprecating odis-3-combiner-errors metric in favor of odis-3-sig-request-errors? I dont see that we are logging any odis-3-combiner-errors

  • Will this PR also include the publishing of the common package? If so, the changeset should be added for the common package. Same goes for the signer package.

packages/phone-number-privacy/combiner/src/common/crypto-clients/bls-crypto-client.ts Show resolved Hide resolved
packages/phone-number-privacy/combiner/src/common/crypto-clients/domain-crypto-client.ts Outdated Show resolved Hide resolved
packages/phone-number-privacy/combiner/src/common/combine.ts Show resolved Hide resolved
packages/phone-number-privacy/combiner/src/common/combine.ts Outdated Show resolved Hide resolved
packages/phone-number-privacy/combiner/src/common/combine.ts Show resolved Hide resolved
packages/phone-number-privacy/combiner/src/common/handlers.ts Outdated Show resolved Hide resolved
packages/phone-number-privacy/combiner/src/pnp/endpoints/sign/action.ts Show resolved Hide resolved
packages/phone-number-privacy/combiner/src/pnp/services/account-services.ts Outdated Show resolved Hide resolved
packages/phone-number-privacy/combiner/src/index.ts Outdated Show resolved Hide resolved
alecps
alecps approved these changes Oct 19, 2023
View reviewed changes
.changeset/loud-dragons-pull.md Outdated Show resolved Hide resolved
@alecps alecps merged commit 101ea6c into main Oct 23, 2023
10 checks passed
@alecps alecps deleted the alecps/odisCombinerk8s branch October 23, 2023 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alecps alecps alecps approved these changes

@soloseng soloseng soloseng approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Modify ODIS Combiner to run in container Add Prometheus metrics to ODIS Combiner code
4 participants
@aaronmgdr @alecps @soloseng @alvarof2